Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

PEP 677: Runtime Behavior Specification #2237

Merged
merged 10 commits into from
Jan 18, 2022
Merged

PEP 677: Runtime Behavior Specification #2237

merged 10 commits into from
Jan 18, 2022

Conversation

stroxler
Copy link
Contributor

I had forgotten to port the detailed spec from
https://docs.google.com/document/d/15nmTDA_39Lo-EULQQwdwYx_Q1IYX4dD5WPnHbFG71Lk/edit

(It was originally separate because I was concerned there might be too much debate and thrash to put it in the PEP, but that did not turn out to be true).

@stroxler stroxler requested a review from gvanrossum as a code owner January 13, 2022 17:07
gvanrossum
gvanrossum reviewed Jan 13, 2022
View reviewed changes
pep-0677.rst Outdated Show resolved Hide resolved
pep-0677.rst Outdated Show resolved Hide resolved
pep-0677.rst Outdated Show resolved Hide resolved
pep-0677.rst Outdated Show resolved Hide resolved
pep-0677.rst Outdated Show resolved Hide resolved
pep-0677.rst Outdated Show resolved Hide resolved
pep-0677.rst Outdated Show resolved Hide resolved
pep-0677.rst Outdated Show resolved Hide resolved
pep-0677.rst Outdated Show resolved Hide resolved
pep-0677.rst Outdated Show resolved Hide resolved
@stroxler
Copy link
Contributor Author

Updates:

  • Flatten the proposal somewhat:
    • Use an is_async flag so we only need one callable type
    • Use Ellipses to indicate AnyArguments
  • Use python pseudocode where possible, English is neither concise nor specific enough
  • Add a runtime section to rejected alternatives discussing other choices we could make
    • I'd be happy to put up a second PR flipping things if we prefer one of the alternatives

JelleZijlstra
JelleZijlstra reviewed Jan 14, 2022
View reviewed changes
pep-0677.rst Outdated Show resolved Hide resolved
JelleZijlstra
JelleZijlstra reviewed Jan 14, 2022
View reviewed changes
pep-0677.rst Outdated Show resolved Hide resolved
@stroxler @JelleZijlstra
Update pep-0677.rst
d46ec9c 
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
@gvanrossum
Copy link
Member

gvanrossum commented Jan 14, 2022 •
edited
Loading

I’m going to have to take a backseat for now, but one source of inspiration could be the ’inspect’ module.

@stroxler
Copy link
Contributor Author

Updates:

  • Switch to an inspect-like approach where arguments is either Ellipses or a tuple of thinks that know their "kind"
  • Change even more of the discussion to use Python pseudocode rather than text
  • Update the "Rejected Alternatives" section, including why we use "arguments" rather than "parameters"

@stroxler stroxler force-pushed the runtime-specification branch from 424099b to e8c71b8 Compare January 15, 2022 19:47
gvanrossum
gvanrossum reviewed Jan 18, 2022
View reviewed changes
Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Some of my more substantive comments can wait for the python-dev discussion if you prefer.)

pep-0677.rst Outdated Show resolved Hide resolved
pep-0677.rst Outdated Show resolved Hide resolved
pep-0677.rst Outdated Show resolved Hide resolved
pep-0677.rst Outdated
kind: CallableTypeArgumentKind
annotation: typing.Type | typing.TypeVar | typing.ParamSpec

class CallableTypeArgumentKind(Enum):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes for very long enum names. Although perhaps @global_enum could help?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't know @global_enum was a thing!

Should https://docs.python.org/3.11/library/enum.html say something about it?

I had to do a code search on CPython to confirm it exists, search engines appear to be unaware of it.

pep-0677.rst Outdated Show resolved Hide resolved
pep-0677.rst Outdated Show resolved Hide resolved
pep-0677.rst Show resolved Hide resolved
pep-0677.rst Show resolved Hide resolved
@stroxler
Copy link
Contributor Author

Updates:

  • Add some type annotations, remove others
  • Adjust wording in a few places as suggested in code review
  • Add missing get_parameters function to the backward-compatible API section

gvanrossum
gvanrossum reviewed Jan 18, 2022
View reviewed changes
Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Rest looks good, just need to fix two declarations.)

pep-0677.rst Outdated Show resolved Hide resolved
gvanrossum
gvanrossum approved these changes Jan 18, 2022
View reviewed changes
Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, merging now.

@gvanrossum gvanrossum merged commit 4a3a993 into python:main Jan 18, 2022
@erlend-aasland erlend-aasland mentioned this pull request May 13, 2022
Closed
@erlend-aasland erlend-aasland mentioned this pull request Jun 27, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JelleZijlstra JelleZijlstra JelleZijlstra left review comments

@gvanrossum gvanrossum gvanrossum approved these changes

Assignees
No one assigned
Labels
CLA signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@stroxler @gvanrossum @JelleZijlstra @the-knights-who-say-ni